Skip to content

Optimize the SqlGuid struct#72549

Merged
jeffhandley merged 15 commits into
dotnet:mainfrom
MichalPetryka:sqlguid
Jan 30, 2023
Merged

Optimize the SqlGuid struct#72549
jeffhandley merged 15 commits into
dotnet:mainfrom
MichalPetryka:sqlguid

Conversation

@MichalPetryka

@MichalPetryka MichalPetryka commented Jul 20, 2022

Copy link
Copy Markdown
Contributor

Optimizes the SqlGuid struct according to the idea
from #51836.

@MichalPetryka
Optimize the SqlGuid struct
e07c937 
Optimizes the SqlGuid struct according to the idea
from dotnet#51836.
@ghost ghost added area-System.Data.SqlClient community-contribution Indicates that the PR has been added by a community member labels Jul 20, 2022
@ghost

ghost commented Jul 20, 2022

Copy link
Copy Markdown

Tagging subscribers to this area: @cheenamalhotra, @David-Engel
See info in area-owners.md if you want to be subscribed.

Issue Details

Optimizes the SqlGuid struct according to the idea
from #51836.

Author: MichalPetryka
Assignees: -
Labels:

area-System.Data.SqlClient
Milestone: -

@MichalPetryka MichalPetryka mentioned this pull request Jul 20, 2022
Closed
GrabYourPitchforks
GrabYourPitchforks reviewed Jul 20, 2022
View reviewed changes
Comment thread src/libraries/System.Data.Common/src/System/Data/SQLTypes/SQLGuid.cs Outdated
Comment thread src/libraries/System.Data.Common/src/System/Data/SQLTypes/SQLGuid.cs Outdated
Comment thread src/libraries/System.Data.Common/src/System/Data/SQLTypes/SQLGuid.cs Outdated
Comment thread src/libraries/System.Data.Common/src/System/Data/SQLTypes/SQLGuid.cs
Comment thread src/libraries/System.Data.Common/src/System/Data/SQLTypes/SQLGuid.cs Outdated
Comment thread src/libraries/System.Data.Common/src/System/Data/SQLTypes/SQLGuid.cs Outdated
Comment thread src/libraries/System.Data.Common/src/System/Data/SQLTypes/SQLGuid.cs Outdated
void ISerializable.GetObjectData(SerializationInfo si, StreamingContext sc)
{
ArgumentNullException.ThrowIfNull(si);
si.AddValue("m_value", ToByteArray());

@GrabYourPitchforks GrabYourPitchforks Jul 20, 2022

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the behavior of ToByteArray is reverted, this will have to change.

Wraith2
Wraith2 reviewed Jul 20, 2022
View reviewed changes
Comment thread src/libraries/System.Data.Common/src/System/Data/SQLTypes/SQLGuid.cs Outdated
GrabYourPitchforks
GrabYourPitchforks reviewed Jul 21, 2022
View reviewed changes
Comment thread src/libraries/System.Data.Common/src/System/Data/SQLTypes/SQLGuid.cs Outdated
Comment thread src/libraries/System.Data.Common/src/System/Data/SQLTypes/SQLGuid.cs Outdated
@Wraith2 Wraith2 mentioned this pull request Jul 23, 2022
Merged
madelson
madelson reviewed Aug 12, 2022
View reviewed changes
Comment thread src/libraries/System.Data.Common/src/System/Data/SQLTypes/SQLGuid.cs Outdated
@ghost ghost closed this Sep 11, 2022
@ghost

ghost commented Sep 11, 2022

Copy link
Copy Markdown

Draft Pull Request was automatically closed for 30 days of inactivity. Please let us know if you'd like to reopen it.

@MichalPetryka

MichalPetryka commented Sep 11, 2022

Copy link
Copy Markdown
Contributor Author

@David-Engel The bot closed this, could it be reopened?

@stephentoub stephentoub reopened this Sep 11, 2022
@ghost

ghost commented Oct 12, 2022

Copy link
Copy Markdown

Draft Pull Request was automatically closed for 30 days of inactivity. Please let us know if you'd like to reopen it.

@ghost ghost closed this Oct 12, 2022
@teo-tsirpanis teo-tsirpanis reopened this Oct 18, 2022
@ghost ghost closed this Nov 17, 2022
@ghost

ghost commented Nov 17, 2022

Copy link
Copy Markdown

Draft Pull Request was automatically closed for 30 days of inactivity. Please let us know if you'd like to reopen it.

@teo-tsirpanis teo-tsirpanis reopened this Nov 17, 2022
@MichalPetryka
MichalPetryka marked this pull request as ready for review November 17, 2022 23:10
@MichalPetryka

MichalPetryka commented Nov 17, 2022

Copy link
Copy Markdown
Contributor Author

@David-Engel @Wraith2 So what's the status on this?

@build-analysis build-analysis Bot mentioned this pull request Nov 18, 2022
Open
Rob-Hague
Rob-Hague reviewed Nov 18, 2022
View reviewed changes
Comment thread src/libraries/System.Data.Common/src/System/Data/SQLTypes/SQLGuid.cs
David-Engel
David-Engel approved these changes Dec 13, 2022
View reviewed changes

@David-Engel David-Engel left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, this fell off my radar in draft so long and the last mention got buried under other emails. I'm good with the changes.

@David-Engel

David-Engel commented Dec 13, 2022

Copy link
Copy Markdown
Contributor

If @GrabYourPitchforks is good with the changes, they can be merged.

@danmoseley

danmoseley commented Jan 23, 2023

Copy link
Copy Markdown
Contributor

@jeffhandley is there anyone that can help look at this one

jeffhandley
jeffhandley requested changes Jan 24, 2023
View reviewed changes
Comment thread src/libraries/System.Data.Common/src/System/Data/SQLTypes/SQLGuid.cs
Comment thread src/libraries/System.Data.Common/src/System/Data/SQLTypes/SQLGuid.cs Outdated
@ghost ghost added needs-author-action An issue or pull request that requires more info or actions from the author. and removed needs-author-action An issue or pull request that requires more info or actions from the author. labels Jan 24, 2023
@build-analysis build-analysis Bot mentioned this pull request Jan 29, 2023
Closed
jeffhandley
jeffhandley approved these changes Jan 30, 2023
View reviewed changes
Comment thread src/libraries/System.Data.Common/src/System/Data/SQLTypes/SQLGuid.cs Outdated
@jeffhandley

jeffhandley commented Jan 30, 2023

Copy link
Copy Markdown
Member

The failing CI leg is unrelated:

@jeffhandley
jeffhandley merged commit bf5afd4 into dotnet:main Jan 30, 2023
@jeffhandley

jeffhandley commented Jan 30, 2023

Copy link
Copy Markdown
Member

Thank you for this contribution, @MichalPetryka! This change will be included in .NET 8 Preview 1.

@jeffhandley jeffhandley added this to the 8.0.0 milestone Jan 30, 2023
@Wraith2 Wraith2 mentioned this pull request Feb 22, 2023
Closed
1 task
@ghost ghost locked as resolved and limited conversation to collaborators Mar 1, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@GrabYourPitchforks GrabYourPitchforks GrabYourPitchforks left review comments
@jeffhandley jeffhandley jeffhandley approved these changes
@David-Engel David-Engel David-Engel approved these changes
+3 more reviewers
@madelson madelson madelson left review comments
@Rob-Hague Rob-Hague Rob-Hague left review comments
@Wraith2 Wraith2 Wraith2 left review comments
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

area-System.Data.SqlClient community-contribution Indicates that the PR has been added by a community member

Projects

None yet

Milestone

8.0.0

Development

Successfully merging this pull request may close these issues.

10 participants

@MichalPetryka @David-Engel @danmoseley @jeffhandley @madelson @GrabYourPitchforks @Rob-Hague @Wraith2 @stephentoub @teo-tsirpanis